[Performance] HBG: up to 99% host-side overhead reduction - #1659
[Performance] HBG: up to 99% host-side overhead reduction#1659SergioMartin86 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe host runtime now uses uniquely owned shared-memory storage, initializes only control regions, and uploads the live payload prefix plus complete control segments to the device. ChangesShared-memory staging
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/a2a3/runtime/host_build_graph/host/runtime_maker.cpp`:
- Around line 537-548: Validate total_tasks before computing payload_prefix_end
or performing relocation/copy operations: require it to be non-negative and no
greater than eff_task_window_sizes[0]. Reject invalid values early, preserving
the existing copy behavior only for valid task counts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e9f8a9e0-e5d7-420b-a483-e1085fb0681e
📒 Files selected for processing (1)
src/a2a3/runtime/host_build_graph/host/runtime_maker.cpp
184c2cd to
ed90f0c
Compare
677289d to
ab534b5
Compare
…tasks HBG's per-dispatch wall is 96-99.6% host bind, and bind was dominated by rebuilding and H2D-uploading full, ring-sized host structures every run -- the shared-memory mirror and the ~20 MB prebuilt runtime arena -- even though a run touches a tiny fraction. Their sizes track ring capacity (task window, 65536-slot pools), not the workload. The device boots scheduler-only and reads no slot past total_tasks, so both are made init-on-write and shipped bounded to the task count. Shared memory: - descriptors and payloads are written per task at submit; slot_states and completion_flags are reset per slot in orch::prepare_task as it is claimed, dropping the window-wide reset loop in init_header_per_ring; only the header is zeroed on the host; each segment is H2D-uploaded bounded to [0, total_tasks). Runtime arena: - skip uploading the orchestrator block (fanin_seen_epoch / scope / tensormap, ~8.5 MB): host-only dep-computation scratch the AICPU scheduler never reads. - skip the build's O(capacity) ready-queue slot init (headers only) and seed each big queue's slots post-orchestration, uploading only that live prefix; safe because relocate_host_orch_image does not walk the ready queues (roots ride in the SM; the device boot scan classifies), so uninitialized slots are never read between. - the prefix is min(total_tasks + 1, capacity). A queue takes at most total_tasks pushes, so its enqueue_pos never exceeds total_tasks, but pop_batch_tagged reads one slot past dequeue_pos -- the slot at enqueue_pos -- to detect the empty boundary, and a batched dequeue that finds a stale (too-large) Vyukov sequence there spins forever. Seeding one sentinel slot past the task count gives that boundary read a valid empty sequence; the ring-sized tail beyond it is never touched. - drop the redundant per-entry stores in the tensormap reset (the preceding memset already zeroes the link pointers and producer_task_id). Every scheduler-read field is initialized at submit, so nothing depends on the removed blanket zeros; reads are bounded by current_task_index. total_tasks is range-checked before it sizes the copies. Single-ring (PTO2_MAX_RING_DEPTH == 1). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqeALZTPEnDXTnbYfcnPfq
ab534b5 to
f4c36af
Compare
Human Summary
The host side of HBG was taking 20x more time than the device side. This was due to an unnecessary resetting (zeroing) and copying of the entire scheduling workspace. This PR reduces the H2D transfer to the minimal required information, and the work structures are initialized-on-use, rather than pre-zeroed.
AI Summary
PR: host_build_graph — init-on-write SM + runtime arena, bounded to total_tasks
Branch
hbg-sm-init-on-writeoffupstream/main(b7141748). One commit, +121/−34across 6 files.
TL;DR
HBG's per-dispatch wall is 96–99.6% host
bind, andbindwas dominated byrebuilding and H2D-uploading full, ring-sized host structures every run — the
shared-memory mirror and the ~20 MB prebuilt runtime arena — even though a run touches
a tiny fraction of either. Their sizes track ring capacity (task window, 65536-slot pools),
not the workload. The device boots scheduler-only and reads no slot past
total_tasks, soboth are made init-on-write and shipped bounded to the task count.
Result — host
bind, A/B vs upstream (a2a3, exclusive card, 100-round median):Device time is unchanged (~0.05–0.1 ms throughout) — the entire win drops into
per-dispatch latency.
bindhas ~±1 ms run-to-run jitter, but against 77–403 ms baselinesthe reductions are unambiguous.
Problem
Splitting
bind(via[STRACE]markers) put nearly all of it inrun_host_orchestration/bind_callable_to_runtime_impl, in two structures that are both sized by ring capacity andrebuilt+reuploaded every run:
host_sm_buf(sm_size, 0)): 81 MB (default ring) to 651 MB(4 GB ring), of which ~97% is the payload segment.
tensormap (65536 entries).
A run uses a few dozen of the 16k–131k slots, so almost all of the alloc/zero/init and the
uploads is work on capacity that is never touched.
Fix
The device reads no slot past
total_tasks. So:Shared memory — descriptors and payloads are written per task at submit; slot_states and
completion_flags are reset per slot in
orch::prepare_taskas it is claimed (dropping thewindow-wide reset loop in
init_header_per_ring); only the header is zeroed on the host;each segment is H2D-uploaded bounded to
[0, total_tasks).Runtime arena —
host-only dep-computation scratch the AICPU scheduler never reads.
slots post-orchestration, uploading only that live prefix instead of the ring-sized tail.
Safe:
relocate_host_orch_imagedoes not walk the ready queues (roots ride in the SM; thedevice boot scan classifies), so uninitialized slots are never read in between.
min(total_tasks + 1, capacity). A queue takes at mosttotal_taskspushes, but
PTO2ReadyQueue::pop_batch_taggedreads one slot pastdequeue_pos— the slotat
enqueue_pos— to detect the empty boundary; a batched dequeue that finds a stale(too-large) Vyukov sequence there spins forever. So the prefix seeds one sentinel slot past
the task count, giving the boundary read a valid "empty" sequence; the tail beyond it is
never touched. (Capacity is a fixed 65536 while the task window can exceed it, so the prefix
is clamped.)
memsetalreadyzeroes the link pointers and
producer_task_id).total_tasksis range-checked before it sizes the copies. Single-ring (PTO2_MAX_RING_DEPTH == 1).The
+1sentinel slot — why bounding a Vyukov queue to exactlytotal_tasksdeadlocksThis is the non-obvious footgun in bounding the ready queues, worth calling out for anyone
who touches this code or applies the same "ship a live prefix" pattern to another MPMC queue.
A first cut seeded and shipped each ready queue's slots
[0, total_tasks)— the reasoningbeing "a queue takes at most
total_taskspushes, and reads no slot past its push count." Thefirst half is true; the second is not.
PTO2ReadyQueue::pop_batch_taggedsizes a batch byscanning forward from
dequeue_posuntil it hits a slot whose Vyukov sequence marks it empty— and that terminating read lands on the slot at
enqueue_pos, i.e. one slot past the lastone actually pushed. When a queue holds every task,
enqueue_posreachestotal_tasks, sothe boundary read touches slot
total_tasks— the first slot the[0, total_tasks)prefix didnot seed or upload.
On a fresh device that slot happens to read as empty and nothing breaks — which is why this
passed single-shot runs. On reuse of the persistent device arena (2nd+ invocation of any
callable), that slot still holds a stale, too-large sequence from a prior run.
pop_batchreads it, computes
diff > 0, and — per its contract — treats that as "a concurrent produceris mid-push here, retry" and spins the outer loop forever. The last ready task is never
dequeued; the scheduler latches
SCHEDULER_TIMEOUT(surfaced host-side as507018). Itreproduced intermittently under pytest-xdist (~2 hangs/pass) and was invisible to golden runs
that don't reuse across differently-sized callables.
Fix: seed and ship the prefix as
min(total_tasks + 1, capacity)— one sentinel slot pastthe task count, carrying its normal empty sequence so the boundary read resolves to "empty" and
pop_batchbreaks cleanly.pop_batchnever scans past that first empty slot, and per-queueenqueue_pos ≤ total_tasks, so a single sentinel is sufficient for every queue and every splitof tasks across shapes. The tail beyond it is genuinely never read.
Takeaway for future bounding work: a lock-free queue's read set can extend one element past
its write set. Bound to the read set (
pushes + 1), not the write set (pushes).Why it is safe
Every scheduler-read field is explicitly initialized at submit, so nothing depends on the
removed blanket zeros; reads are bounded by
current_task_index. The orchestrator block ishost-only (verified: zero AICPU-scheduler references); relocation never walks the ready
queues; the tensormap-reset change is a pure dedup of what
memsetalready wrote.Validation
100-round golden × 4 workloads PASS (bgemm, matmul, vector, paged_attention — the last with
its 131072-slot window, exercising the capacity clamp and cross-run queue reuse).
Cross-run reuse is stress-tested under pytest-xdist (one worker per device, the whole HBG
suite, multiple back-to-back passes across 6 devices): 0 scheduler stalls / 0 op-execute
timeouts. This is the regression barrier for the
pop_batchboundary slot — before thetotal_tasks + 1sentinel, the same stress produced an intermittentSCHEDULER_TIMEOUT(≈2 hangs/pass) on the second-and-later invocation of a callable that reused the persistent
device arena.
Full HBG a2a3 scene-test suite green except one failure —
run_stream_reuse::test_depth_two_slots_own_separate_resources(slot-1 GM heap bankuncommitted) — which reproduces identically on clean
upstream/main, i.e. pre-existing andunrelated (arena-bank path, not this change).
Scope / not done
host_build_graphonly. Orthogonal to Add Graph Execution to host_build_graph #1444 (graph execution): that shrinks what goesinto the SM; this fixes the SM/arena buffer lifecycle. They compound.
print_statsscans the whole entry pool and dereferences entries, and the pool has arecycling free-list, so bounding its reset is high-risk for a sub-noise gain. The 8 MB
memsetstays.